Repository navigation
Conversation
Added ability to register driver specific classes.
Tests don't currently work due to mssql server not running on my computer.
RECREATE will drop (if needed) before creating
…on the PdoPgsql class.
Made extension loading work and implemented exceptions.
|
I see |
|
Thanks. I saw and fixed them in ext/pdo_mysql/tests/subclasses/pdomysql_001.phpt Did you see any others? |
|
You could also change it for |
Girgias
left a comment
Girgias
left a comment
There was a problem hiding this comment.
Did a cursory review (and messed up by marking this as ready...).
Seems this requires a rebase to retrigger CI.
| memcpy(&pdosqlite_dbh_object_handlers, &std_object_handlers, sizeof(zend_object_handlers)); | ||
| pdosqlite_dbh_object_handlers.offset = XtOffsetOf(pdo_dbh_object_t, std); | ||
| pdosqlite_dbh_object_handlers.free_obj = pdo_dbh_free_storage; | ||
| pdosqlite_dbh_object_handlers.clone_obj = NULL; |
There was a problem hiding this comment.
You shouldn't need to specify the clone handler IIRC as it is NULL by default? Same as the compare handler.
There was a problem hiding this comment.
Removed the clone_obj. Not sure what you meant for the compare handler, as that is not set to NULL.
| PHP_METHOD(PDO, __construct) | ||
|
|
||
| #define MAX_PDO_SUB_CLASSES 64 | ||
| static int number_of_pdo_driver_class_entries = 0; |
| // could this ever happen? | ||
| if (driver->driver_name == NULL) { | ||
| zend_throw_exception_ex(php_pdo_get_exception(), 0, "Driver name is NULL"); | ||
| RETURN_THROWS(); | ||
| } |
There was a problem hiding this comment.
I would ZEND_ASSERT() this as not having a name is a bug in the implementation of the driver.
There was a problem hiding this comment.
so like:
ZEND_ASSERT((driver->driver_name != NULL) && "PDO driver name is null");
?
| // I chose not to, as that would cause BC break and require a lot of | ||
| // downstream work. | ||
| typedef struct { | ||
| char *driver_name; |
There was a problem hiding this comment.
I wonder if it makes sense to have this be a zend_string that is interned by the driver on startup. But that can be a future improvement.
| /** | ||
| * @var int | ||
| * @cname PDO_PGSQL_ATTR_DISABLE_PREPARES | ||
| */ | ||
| public const ATTR_DISABLE_PREPARES = UNKNOWN; |
There was a problem hiding this comment.
@kocsismate does it make sense to move the type information into the constant declaration now that we have typed constants?
| if (!tmp) { | ||
| // TODO - exception | ||
| php_error_docref(NULL, E_WARNING,"Failed to escape identifier"); | ||
| RETURN_FALSE; | ||
| } |
There was a problem hiding this comment.
Obviously this has a TODO but this currently will fail ZPP return type checks
| /** | ||
| * @var int | ||
| * @cname SQLITE_DETERMINISTIC | ||
| */ | ||
| public const DETERMINISTIC = UNKNOWN; |
There was a problem hiding this comment.
Ditto same comment about moving the type info into the constant declaration.
|
Closed in preference for #11740 |
This is just a initial commit, to trigger the full PHP test suite. Obviously needs an RFC writing.
For the record, I know some code is in the wrong place currently. Will put stuff in the right place before changing the PR from draft.